feat: Integrate all routes from nutripatrol's API #991

Valimp · 2024-11-13T10:58:46Z

What

Add nutripatrol's routes to the dart SDK

Fixes bug(s)

#885

monsieurtanuki

Hi @Valimp!
Please have a look at my comments.

monsieurtanuki · 2024-11-13T15:59:18Z

example/lib/main.dart

Quickly said: test your code in test files, not in main.

monsieurtanuki · 2024-11-13T15:59:44Z

lib/openfoodfacts.dart

You'll probably need to add the other nutripatrol classes.

And nutripatrol_types.dart

lib/src/nutripatrol/create_flag.dart

lib/src/nutripatrol/get_ticket.dart

lib/src/nutripatrol_api_client.dart

monsieurtanuki

Hi @Valimp!
The code must pass the analysis tests.

g123k · 2025-01-10T17:08:38Z

@Valimp will have his midterms next week, that's why I've fixed the main warnings.
However the enum NutripatrolSource is declared twice.

Other than that, I think you can review the PR @monsieurtanuki

monsieurtanuki · 2025-01-10T18:07:26Z

@Valimp will have his midterms next week
Other than that, I think you can review the PR

@g123k As you may know currently I don't have a computer, so code reviews are sometimes acrobatic and I don't want to review too often.
Besides, I already had a quick look at the code and I saw several things to fix. If @Valimp is busy, I don't see the point of reviewing now instead of when @Valimp is available.
Unless you're in a hurry and want to fix the code yourself.

monsieurtanuki

Hi @Valimp!
Please have a look at my comments.

monsieurtanuki · 2025-01-28T17:43:13Z

example/lib/main.dart

Please don't use this file. Nobody does.
Please create unit tests for your new classes.

lib/openfoodfacts.dart

monsieurtanuki · 2025-01-28T17:46:22Z

lib/openfoodfacts.dart

 export 'src/model/product_type.dart';
 export 'src/model/product_type_filter.dart';


Some confusion with these guys: one is moved and copied, the other is not.

lib/src/nutripatrol/get_flag.dart

lib/src/nutripatrol/create_flag_request.dart

lib/src/nutripatrol/create_flag.dart

monsieurtanuki

Hi @Valimp!
Please have a look at my comments.

test/api_nutripatrol_create_flag.dart

monsieurtanuki · 2025-02-08T15:41:22Z

test/api_nutripatrol_create_flag.dart

+import 'package:test/test.dart';
+
+void main() {
+  setUpAll(() {


We don't use it in the other tests. Do you really need it? If so please add a specific comment.

monsieurtanuki · 2025-02-08T15:42:21Z

lib/src/utils/nutripatrol_types.dart

@@ -0,0 +1,48 @@
+import 'package:json_annotation/json_annotation.dart';
+
+enum NutripatrolSource {


I would

extend OffTagged

put each enum in a specific file

add "Nutripatrol" and remove "Enum" in your comments

But that's not mandatory.

lib/src/nutripatrol_api_client.dart

monsieurtanuki · 2025-02-08T15:52:19Z

lib/openfoodfacts.dart

And nutripatrol_types.dart

monsieurtanuki · 2025-02-08T15:52:53Z

lib/src/model/old_product_result.g.dart

No, you should NOT change that file, please.

lib/src/nutripatrol/create_flag.dart

lib/src/nutripatrol/get_flag.dart

monsieurtanuki · 2025-02-08T15:55:51Z

test/api_nutripatrol_create_flag.dart

You should test the other methods of Nutripatrol.
For instance, list the tickets and find the one you've just created.

teolemon · 2025-03-04T15:34:01Z

@monsieurtanuki @g123k can we finally merge this ?

monsieurtanuki · 2025-03-05T14:13:29Z

lib/src/model/old_product_result.g.dart

Please keep the comment as it was.

monsieurtanuki · 2025-03-05T14:14:17Z

test/api_nutripatrol_create_flag.dart

+import 'dart:math';
+
+import 'package:openfoodfacts/openfoodfacts.dart';
+import 'package:openfoodfacts/src/nutripatrol/nutripatrol_types.dart';


It's not supposed to be there, but in the list of exported dart files.

monsieurtanuki · 2025-03-05T14:16:14Z

lib/src/nutripatrol_api_client.dart

+      return MaybeError<CreateNutripatrolFlag>.value(
+          CreateNutripatrolFlag.fromJson(decodedResponse));
+    }
+    return MaybeError<CreateNutripatrolFlag>.responseError(response);


Please use try / catch for all your MaybeError statements, cf. prices.

monsieurtanuki · 2025-03-05T14:17:19Z

@monsieurtanuki @g123k can we finally merge this ?

@teolemon Quickly said: no.

there are many unresolved conversations
in the enum types (nutripatrol_types.dart) a label field has been introduced instead of the widely used (and suggested) offTag
there are a couple of unwanted lines to remove in old_product_result.g.dart - this file isn't supposed to change, the specific comment is meant to be kept
I suspect that nutripatrol_types.dart isn't exported
suggested tests are missing, like testing if whatever we created can be retrieved afterwards

github-actions bot assigned Valimp Nov 13, 2024

Valimp changed the title ~~Integrate all routes from nutripatrol's API~~ feat: Integrate all routes from nutripatrol's API Nov 13, 2024

monsieurtanuki reviewed Nov 13, 2024

View reviewed changes

teolemon added the 🛡️ Nutri-Patrol label Nov 26, 2024

g123k marked this pull request as ready for review January 10, 2025 09:31

g123k requested a review from a team as a code owner January 10, 2025 09:31

g123k requested a review from monsieurtanuki January 10, 2025 09:31

monsieurtanuki requested changes Jan 10, 2025

View reviewed changes

Valimp and others added 13 commits January 22, 2025 14:20

feat: get tickets for nutripatrol

74d2d47

fix: get tickets should be a list

9a8f218

fix: use enum to type datas

934bb33

feat: create flag type

a499d84

fix: add generated files

9713eb4

feat: add getTicket and getTickets in the sdk

83c0c64

feat: add get tickets route

117de10

feat: create ticket route

7a58401

feat: change ticket status route

e2c1017

fix: format test

c253ad4

Fixes

3f20cca

feat: integrate create flag route

290d3fa

fix: refactor source enum

7525254

Valimp force-pushed the get-tickets-sdk branch from 5df1fec to 7525254 Compare January 22, 2025 13:22

Valimp requested a review from monsieurtanuki January 27, 2025 09:53

monsieurtanuki reviewed Jan 28, 2025

View reviewed changes

Valimp added 4 commits January 29, 2025 14:08

fix: refactor enums and fix type variables

e5507fa

fix: refactor nutripatrol types and nutripatrol api client

affc5d6

fix: delete update class

8f3fd4f

fix: make status code match

0235932

Valimp added 3 commits February 5, 2025 14:12

feat: make unit test on create flag route

abc2777

remove unused features

03b4697

fix: remove main previous commit

0b8e697

Valimp requested a review from monsieurtanuki February 6, 2025 09:37

fix: label for nutripatrol types

6ea970b

monsieurtanuki reviewed Feb 8, 2025

View reviewed changes

Valimp and others added 2 commits March 4, 2025 15:50

fix: make tests for get ticket by id and get tickets

7dec703

Merge branch 'master' into get-tickets-sdk

753a4fa

monsieurtanuki reviewed Mar 5, 2025

View reviewed changes

lib/src/model/old_product_result.g.dart

Copy link

Contributor

monsieurtanuki Mar 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep the comment as it was.

monsieurtanuki reviewed Mar 5, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Integrate all routes from nutripatrol's API #991

feat: Integrate all routes from nutripatrol's API #991

Valimp commented Nov 13, 2024

monsieurtanuki left a comment

monsieurtanuki Nov 13, 2024

monsieurtanuki Nov 13, 2024

monsieurtanuki Feb 8, 2025

monsieurtanuki left a comment

g123k commented Jan 10, 2025

monsieurtanuki commented Jan 10, 2025

monsieurtanuki left a comment

monsieurtanuki Jan 28, 2025

monsieurtanuki Jan 28, 2025

monsieurtanuki left a comment

monsieurtanuki Feb 8, 2025

monsieurtanuki Feb 8, 2025

monsieurtanuki Feb 8, 2025

monsieurtanuki Feb 8, 2025

monsieurtanuki Feb 8, 2025

teolemon commented Mar 4, 2025

monsieurtanuki Mar 5, 2025

monsieurtanuki Mar 5, 2025

monsieurtanuki Mar 5, 2025

monsieurtanuki commented Mar 5, 2025

		export 'src/model/product_type.dart';
		export 'src/model/product_type_filter.dart';

		@@ -0,0 +1,48 @@
		import 'package:json_annotation/json_annotation.dart';

		enum NutripatrolSource {

feat: Integrate all routes from nutripatrol's API #991

Are you sure you want to change the base?

feat: Integrate all routes from nutripatrol's API #991

Conversation

Valimp commented Nov 13, 2024

What

Fixes bug(s)

monsieurtanuki left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monsieurtanuki left a comment

Choose a reason for hiding this comment

g123k commented Jan 10, 2025

monsieurtanuki commented Jan 10, 2025

monsieurtanuki left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monsieurtanuki left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

teolemon commented Mar 4, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monsieurtanuki commented Mar 5, 2025